Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking: Remove sinon.defaultConfig and related modules #2565

Merged
merged 4 commits into from
Nov 2, 2023

Conversation

fatso83
Copy link
Contributor

@fatso83 fatso83 commented Oct 28, 2023

default-config and get-config are leftovers from when Sinon shipped with sinon.test (now the independent NPM module
'sinon-test').

These serve no purpose internally, and really have no purpose but to help sinon-test create a base default. If needed, these can be copied into the sinon-test project. No projects should depend on these (my assumption), but since it is a change of the API we mark it as a breaking change

When working with these I found out that some things had never been documented, so added docs for that.

fixes #2561

@fatso83 fatso83 added this to the v18 milestone Oct 28, 2023
@fatso83 fatso83 marked this pull request as draft October 28, 2023 20:47
@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b5fc367) 96.05% compared to head (441daf4) 96.05%.

❗ Current head 441daf4 differs from pull request most recent head 94ee6d3. Consider uploading reports for the commit 94ee6d3 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2565      +/-   ##
==========================================
- Coverage   96.05%   96.05%   -0.01%     
==========================================
  Files          41       40       -1     
  Lines        1928     1927       -1     
==========================================
- Hits         1852     1851       -1     
  Misses         76       76              
Flag Coverage Δ
unit 96.05% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/create-sinon-api.js 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fatso83 fatso83 marked this pull request as ready for review October 29, 2023 05:52
default-config and get-config are leftovers from when Sinon
shipped with sinon.test (now the independent NPM module
 'sinon-test').

These serve no purpose internally, and really have no purpose
but to help sinon-test create a base default. If needed,
these can be copied into the sinon-test project. No projects
should depend on these (my assumption), but since it is a
change of the API we mark it as a breaking change

fixes sinonjs#2561
It seemed like the the 'injectInto' option
would expose most props by default. This was
not the case. That was formerly hidden by
using the getConfig call that added props
that were never used in the actual implementation.

Added another test to make this more explicit.
Will add docs on this.
This was added in Sinon 0.6 but has never been documented
Copy link
Member

@mroderick mroderick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR! 🥇

@fatso83 fatso83 merged commit 93db3ef into sinonjs:main Nov 2, 2023
7 checks passed
@fatso83 fatso83 deleted the remove-default-config branch November 2, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove unused/meaningless exports and modules from the codebase
2 participants